PDX-493: feat(mcp) — date/datetime/boolean/integer valueClass dispatch (Thread C, PR 1/2)#183
Conversation
…a inferSalesforceValueClass helper RCA: Provar runtime silently discards Salesforce date/datetime fields whose XML emits valueClass="string" — the failure surfaces only when a later validation rule reads the empty field. The existing buildArgumentValue dispatch in src/mcp/tools/testCaseGenerate.ts fell through to a hard-coded valueClass="string" fallback after handling variable / compound / uiTarget / uiLocator cases, so every literal date or boolean argument flowed out untyped. A prior helper (inferValueClass) introduced in 3c1545c that covered the boolean case alone was refactored away in 2d4240c, regressing literal-true/false handling too. Datetime, integer, and explicit org-describe-driven type hints were never covered. Fix: Introduce an exported inferSalesforceValueClass(key, val, fieldTypeHint?) helper that returns one of 'date' | 'datetime' | 'boolean' | 'integer' | 'string'. Detection order: explicit fieldTypeHint wins; then ISO-8601 datetime regex /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/; then ISO-8601 date regex /^\d{4}-\d{2}-\d{2}$/; then literal 'true'/'false'; then integer-only string /^-?\d+$/; else string. buildArgumentValue now calls this helper before the (now-removed) string fallback and emits <value class="value" valueClass="<inferred>">…</value>. The fieldTypeHint param is wired into the helper signature but not yet exposed on the MCP tool input schema — that exposure lands in PDX-492 H2b, which sits behind PDX-491 H2a (provar_org_describe). Until then callers omit the hint and detection falls through to the regex layer. Tool description and docs/mcp.md argument-conventions table updated to spell out the auto-detection rules. Tests: New PDX-493 — inferSalesforceValueClass helper block exercises the helper directly across all six branches (datetime with fractional seconds + zone, plain date, true, false, positive integer, negative integer, plain string), the edge case where a short numeric string like "12" must resolve to integer not date (date regex requires full ISO yyyy-mm-dd), strings that look like dates but miss the ISO format must stay string, and explicit fieldTypeHint wins over format detection in all three directions (string-shape + date hint, date-shape + string hint, integer-shape + boolean hint). A second PDX-493 — valueClass emission in generated XML block round-trips through provar_testcase_generate and asserts the inferred attribute reaches the emitted XML for date, datetime, boolean (true and false), positive integer, negative integer, and plain string. Validation: node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts" — 1172 passing / 0 failing; yarn compile clean; yarn lint clean (lint:script-names + lint). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Quality Orchestrator🟢 LOW · 🧪 Tests to Run · Running 1 of 51 tests
▶ Run commandnpx vitest run \
unit/mcp/testCaseGenerate.test.ts⚡ quality-orchestrator · |
There was a problem hiding this comment.
Pull request overview
Adds valueClass inference for generated MCP test-case XML so Salesforce-like date, datetime, boolean, and numeric-looking string values are emitted with typed valueClass attributes instead of always falling back to string.
Changes:
- Adds and exports
inferSalesforceValueClass(...)and wires it into argument XML emission. - Adds unit coverage for helper inference and generated XML valueClass output.
- Updates MCP documentation for argument valueClass auto-detection rules.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/mcp/tools/testCaseGenerate.ts |
Adds valueClass inference and updates tool guidance. |
test/unit/mcp/testCaseGenerate.test.ts |
Adds helper and XML emission tests for inferred value classes. |
docs/mcp.md |
Documents the new valueClass inference table and detection order. |
Comments suppressed due to low confidence (3)
src/mcp/tools/testCaseGenerate.ts:406
- This introduces
valueClass="integer", but the repository's canonical step reference documents numeric values asvalueClass="decimal"(for example,docs/PROVAR_TEST_STEP_REFERENCE.md:1337-1338and the data-types checklist at:1428). Please either emit the documented numeric value class here or update the canonical reference with evidence thatintegeris a supported Provar valueClass for these generated arguments.
if (/^-?\d+$/.test(val)) return 'integer';
test/unit/mcp/testCaseGenerate.test.ts:842
- This assertion locks in
valueClass="integer", while the canonical step reference currently documents numeric values asvalueClass="decimal"(docs/PROVAR_TEST_STEP_REFERENCE.md:1337-1338,:1428). Update the expected value class to match the supported Provar numeric type, or add canonical documentation showing thatintegeris valid before testing for it here.
assert.ok(xml.includes('valueClass="integer">42</value>'), `Expected valueClass="integer" for "42"; got: ${xml}`);
test/unit/mcp/testCaseGenerate.test.ts:860
- This also expects
valueClass="integer"for a numeric literal, which conflicts with the canonical step reference'svalueClass="decimal"guidance for numbers (docs/PROVAR_TEST_STEP_REFERENCE.md:1337-1338,:1428). Keep the negative-number case aligned with whichever numeric valueClass is supported.
assert.ok(xml.includes('valueClass="integer">-5</value>'), `Expected valueClass="integer" for "-5"; got: ${xml}`);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fieldTypeHint?: 'date' | 'datetime' | 'boolean' | 'integer' | 'string' | ||
| ): 'date' | 'datetime' | 'boolean' | 'integer' | 'string' { | ||
| if (fieldTypeHint) return fieldTypeHint; | ||
| if (/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/.test(val)) return 'datetime'; |
There was a problem hiding this comment.
Addressed in 8def6af: datetime regex is now end-anchored with optional fractional seconds and timezone — /^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:?\d{2})?$/. Added a positive test for numeric timezone offsets and a negative test that rejects 2026-05-19T10:30:00not-a-zone as plain string.
| | Value matches `^-?\d+$` | `class="value" valueClass="integer"` | Any step | | ||
| | All other values | `class="value" valueClass="string"` | Any step | | ||
|
|
||
| `valueClass` is inferred automatically by `inferSalesforceValueClass(key, val, fieldTypeHint?)`. Detection order: explicit `fieldTypeHint` (wired in a follow-up tool surface — `field_type_hints` param) → ISO-8601 datetime → ISO-8601 date → boolean → integer → string. Provar runtime silently discards date fields emitted as `valueClass="string"`, so always pass date / datetime values in ISO-8601 form. |
There was a problem hiding this comment.
Addressed in 8def6af: docs/mcp.md now lists valueClass="decimal" for the numeric row (one merged row covering 42, -5, 3.14). Matches PROVAR_TEST_STEP_REFERENCE.md lines 1338 and 1428.
| '"true"/"false" → "boolean"; integer-only string (e.g. "42", "-5") → "integer"; otherwise "string". ' + | ||
| 'Pass dates / booleans / integers in those formats — Provar runtime silently discards date fields emitted as valueClass="string".', |
There was a problem hiding this comment.
Addressed in 8def6af: tool-description guidance now says "numeric string → decimal" and explicitly notes there is no separate integer valueClass per the canonical reference. The helper return type was also narrowed to 'date' | 'datetime' | 'boolean' | 'decimal' | 'string'.
| it('returns "integer" for positive integer string', () => { | ||
| assert.equal(inferSalesforceValueClass('Quantity', '42'), 'integer'); | ||
| }); | ||
|
|
||
| it('returns "integer" for negative integer string', () => { | ||
| assert.equal(inferSalesforceValueClass('Delta', '-5'), 'integer'); | ||
| }); | ||
|
|
||
| it('returns "string" for plain text', () => { | ||
| assert.equal(inferSalesforceValueClass('Name', 'Acme Corp'), 'string'); | ||
| }); | ||
|
|
||
| it('returns "integer" not "date" for a short numeric string like "12"', () => { | ||
| // Edge case: the date regex requires the full ISO yyyy-mm-dd form, so a bare "12" | ||
| // is integer, not date. Guards against false-positive date detection on numeric IDs. | ||
| assert.equal(inferSalesforceValueClass('Code', '12'), 'integer'); |
There was a problem hiding this comment.
Addressed in 8def6af: helper tests and XML emission tests now assert decimal for all numeric inputs (42, -5, short numeric 12). Added positive coverage for decimal values 3.14 and -12.5 at both helper and XML levels; integer is removed from the contract entirely.
… (integer → decimal) + anchor datetime regex
RCA: The H3 plan AC specified `valueClass="integer"` for integer-only strings, but
`docs/PROVAR_TEST_STEP_REFERENCE.md` lines 1338 and 1428 are explicit: numbers in
Provar test steps use `valueClass="decimal"` (covering both `42` and `3.14`). No
`integer` valueClass exists in the canonical reference grammar, so the initial
H3 implementation would have emitted XML that diverges from the Provar runtime
contract for every integer field — silently produced bad XML for any numeric
Salesforce field. Separately, the datetime detection regex
`/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}/` was not end-anchored, so trailing
garbage after seconds (e.g. `2026-05-19T10:30:00not-a-zone`) was accepted as a
valid datetime and emitted as `valueClass="datetime"`. The canonical reference
permits optional fractional seconds and timezone — the regex must enforce that
shape explicitly.
Fix: In `inferSalesforceValueClass` (src/mcp/tools/testCaseGenerate.ts):
- Replace the `'integer'` arm of the return-type union with `'decimal'`. Both
the parameter and return type now use the canonical-reference set:
`'date' | 'datetime' | 'boolean' | 'decimal' | 'string'`.
- Broaden the numeric regex from `/^-?\d+$/` to `/^-?\d+(\.\d+)?$/` so it
matches integers and decimals; return `'decimal'` for both.
- Anchor the datetime regex end with explicit optional fractional-seconds and
timezone groups: `/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}(\.\d+)?(Z|[+-]\d{2}:?\d{2})?$/`.
- Update doc-comment to spell out the canonical reference (lines 1338/1428).
Update tool description text (line ~155) to say "numeric string → decimal" and
explicitly call out that there is no separate `integer` valueClass. Update
`docs/mcp.md` argument-conventions table row to match (one merged numeric row
covering `42`, `-5`, `3.14`). Tests:
- Rewrite `integer` assertions in test/unit/mcp/testCaseGenerate.test.ts to
`decimal` (positive int, negative int, short numeric string).
- Add positive coverage for decimals (`3.14`, `-12.5`) at both helper and
XML-emission levels.
- Add datetime end-anchor coverage (numeric timezone offset accepted; trailing
garbage rejected as `string`).
Addresses Copilot review feedback on PR #183.
mrdailey99
left a comment
There was a problem hiding this comment.
Addressed in 8def6af: integer → decimal per PROVAR_TEST_STEP_REFERENCE.md lines 1338 and 1428; datetime regex is now end-anchored with optional fractional seconds and timezone. All 4 inline Copilot findings have inline replies. Tests: 1177 passing, 1 pending (no regressions); added 5 new tests (datetime numeric-offset, datetime trailing-garbage rejection, decimal positive/negative literals at helper + XML levels). yarn compile and yarn lint are clean.
Summary
inferSalesforceValueClass(key, val, fieldTypeHint?)helper insrc/mcp/tools/testCaseGenerate.tsand wiresbuildArgumentValueto use it instead of the prior hard-codedvalueClass="string"fallback. The generator now emitsvalueClass="date"/"datetime"/"boolean"/"integer"/"string"based on a strict detection order: explicitfieldTypeHint→ ISO-8601 datetime → ISO-8601 date → literaltrue/false→ integer-only string → string.inferValueClasshelper from 3c1545c) and additionally covers datetime + integer dispatch that was never previously detected. Provar runtime silently discards Salesforce date fields whose XML emitsvalueClass="string"— this fix prevents the silent-drop failure mode.docs/mcp.mdargument-conventions table updated to spell out the auto-detection rules. No new MCP tool surface or input-schema fields are exposed in this PR — thefieldTypeHintparameter is wired into the helper signature only and lands in the MCP input schema in PDX-492 (H2b, Thread C PR 2/2), which sits behind PDX-491 (H2a,provar_org_describe).Scope and threading
This is Thread C, PR 1 of 2 of the PDX-477 plan ("Categories G/H feedback + validate-typo blindspot"). PDX-492 (H2b) — the parameter exposure on
provar_testcase_generate— depends on PDX-491 (H2a,provar_org_describe) landing first and is intentionally out of scope here.No
warningCodes.tsenum entry is added because this PR does not emit any new warning code in PR 1 — the warning emission ("required field missing", "object not in cache") lands in H2b.Files changed
src/mcp/tools/testCaseGenerate.ts— new exported helper, dispatch wired intobuildArgumentValue, tool description updated.test/unit/mcp/testCaseGenerate.test.ts— two new describe blocks (PDX-493 — inferSalesforceValueClass helperandPDX-493 — valueClass emission in generated XML).docs/mcp.md— argument-conventions table extended with the four new value-shape rows + auto-detection paragraph.Test plan
node_modules/.bin/nyc node_modules/.bin/mocha "test/**/*.test.ts"— 1172 passing / 0 failing (bypasses wireit cache per CLAUDE.md guidance).yarn compile— clean.yarn lint— clean (lint:script-names + lint).yarn build && yarn test) passed.fieldTypeHintoverride directions).provar_testcase_generatefor all six dispatch branches (date, datetime, boolean true, boolean false, positive integer, negative integer, plain string).Not in this PR (flagged for follow-up)
field_type_hintsandrequired_fields_hintparameters onprovar_testcase_generate→ PDX-492 / H2b (Thread C PR 2/2, gated on PDX-491 H2aprovar_org_describe).docs/provar-mcp-public-docs.md,docs/university-of-provar-mcp-course.md) are Provar-team-maintained per CLAUDE.md — flagging here so the Provar team can roll the new valueClass rules into the public docs when they next update.package.json/server.jsonversion bump in this feature PR per the plan's "Coordination chores" rule — version bumps happen in a sweeper PR after each batch of merges.